-
Notifications
You must be signed in to change notification settings - Fork 1
Autonomous Refactor #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The shallow merge was overwriting nested config sections entirely instead of merging them. Now properly merges routers, services, and middlewares when updating an existing app's configuration.
Split renderTemplate into two functions: - renderTemplate() returns raw rendered string - renderTemplateParsed<T>() returns parsed YAML object This eliminates the redundant parse->dump->parse cycle where templates were parsed to YAML, dumped back to string, then parsed again by the caller.
The lastUserData variable and _getLastUserData() function were never used by any tests or production code. Removed to reduce dead code.
When building the combined config, log a warning if multiple apps define the same router, service, or middleware name. This helps catch misconfigurations where one app's config would silently overwrite another's.
if initialization failed, the next line would throw a confusing error. Added ensureBackend() helper that explicitly checks and throws a clear error message if the backend failed to initialize.
Previously cleanupTempFiles() ran on every config flush, causing unnecessary filesystem operations. Now tracks cleanup state and only runs once per output file, resetting when the output file changes.
Changed buildContext and getContextValue to use specific Record<string, ...> types instead of 'any' to satisfy ESLint no-explicit-any rules.
…ad code
1. Fix double rendering issue:
- renderTemplateParsed now returns { raw, parsed } to avoid calling
renderTemplate twice (was rendering once to store, then again to parse)
2. Extract reusable error helper:
- Added getErrorMessage(err) to templateParser.ts
- Used in traefik.ts, templateParser.ts, and validators.ts
- Eliminates duplicate 'err instanceof Error ? err.message : String(err)'
3. Remove dead code:
- Removed lastRendered variable (never used)
- Removed _getLastRendered() function (never called from tests)
- Removed duplicate debug log in makeAppConfig (templateParser already logs)
Define a dedicated Context type and use safe unknown-based lookup in getContextValue to satisfy TypeScript without resorting to 'any'.
- Add with and - Use helpers across parser, validators, and manager - Minor readability improvements
- Remove duplicate ComposeFileReference from docker.d.ts (use provider's version) - Remove TraefikConfigYamlFormat re-export from types/index.ts (keep in backend) - Remove duplicate FieldData type from api/types.ts - Extract shared getClientIP utility from auth.ts and logging.ts - Clean up config.ts comments (fix typos, improve clarity) - Remove unnecessary re-export in templateParser.ts
- Reorganize logger.ts imports (group and move type import to top) - Add JSDoc comments to logging functions - Clean up verbose comments and simplify code - Export getClientIP from middleware index - Simplify consoleFormat data source selection
- Reorganize config.ts imports at top - Extract VALID_BACKENDS as const array - Add JSDoc comments to exported functions - Convert BackendModule and BackendStatus to exported interfaces - Add JSDoc comments to backend plugin functions
- Move detectCollisions import to top of file with other imports - Remove redundant section headers (Flush Debouncing) - Simplify comments on module state variables
- Remove unused xMagicProxySchema alias export - Rewrite backends/readme.md with cleaner formatting and updated references
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR is a major autonomous refactoring by an AI agent that consolidates types, improves code organization, and reduces duplication across the codebase.
Changes:
- Consolidated type definitions by removing duplicates and cleaning up exports
- Extracted shared utilities (like
getClientIP) into dedicated modules - Improved logging infrastructure with better documentation and type safety
- Enhanced Traefik backend with collision detection, helper functions, and improved template parsing
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/types/xmagic.ts |
Removed redundant alias export |
src/types/index.ts |
Cleaned up exports and removed Traefik-specific re-export |
src/types/docker.d.ts |
Removed duplicate ComposeFileReference interface and improved documentation |
src/logging/logger.ts |
Refactored with better documentation, improved type safety, and performance optimizations |
src/config.ts |
Improved structure with constants, better documentation, and type safety |
src/backends/traefik/validators.ts |
Now uses shared getErrorMessage helper |
src/backends/traefik/traefikManager.ts |
Added collision detection, improved merging logic, and temp file cleanup optimization |
src/backends/traefik/traefik.ts |
Switched to renderTemplateParsed, removed debug tracking, uses shared helpers |
src/backends/traefik/templateParser.ts |
Introduced typed context, new renderTemplateParsed function, improved type safety |
src/backends/traefik/helpers.ts |
New shared helper module with getErrorMessage and detectCollisions |
src/backends/readme.md |
Improved documentation with better formatting and clarity |
src/backends/backendPlugin.ts |
Added proper interface exports, improved documentation, better error handling |
src/api/types.ts |
Removed unused FieldData interface |
src/api/middleware/utils.ts |
New module extracting shared getClientIP utility |
src/api/middleware/logging.ts |
Now imports getClientIP from utils |
src/api/middleware/auth.ts |
Now imports getClientIP from utils |
src/api/middleware/index.ts |
Added getClientIP to exports |
test/unit/traefik/user-data-substitution.test.ts |
Updated test expectation for null value serialization |
test/unit/traefik/template-error-handling.test.ts |
Now imports and tests renderTemplateParsed |
|
@copilot please review |
|
@stonegray I've opened a new pull request, #32, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: stonegray <7140974+stonegray@users.noreply.github.com>
…ix config types Co-authored-by: stonegray <7140974+stonegray@users.noreply.github.com>
Co-authored-by: stonegray <7140974+stonegray@users.noreply.github.com>
Address review feedback: enhance type safety and test coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 2 comments.
Summary
Major refactor to improve structure, reduce duplication, and increase maintainability and testability across the codebase. 🔧
What's changed
Consolidated and cleaned up types (removed duplicate ComposeFileReference, simplified types exports). ✅
Unified API utilities: extracted shared getClientIP into utils.ts and cleaned middleware exports. ✅
Simplified and documented logging internals (logger.ts): clearer serialization, overflow guard, and formatting. ✅
Cleaned config.ts (better structure, constants and validation). ✅
Improved backend plugin types and loader interface (exported BackendModule / BackendStatus). ✅
Cleaned Traefik backend: template parsing, manager import/structure, and helpers. ✅
Removed dead / redundant code (unused alias export, extra re-exports) and updated backend docs. 🧹
Validation
✅ ESLint: clean (no new lint errors)
✅ Tests: all tests pass (203 tests)
✅ Build: TypeScript compiles successfully
Impact & compatibility
No breaking changes to public runtime APIs (kept backwards-compatible).⚠️ Internal module APIs were reorganized — reviewers should check any external integrations that import internal types directly (rare).
Branch: refactor/autonomous — merged into dev.
Files / areas to review (high value)
src/types/* (type consolidation)
src/logging/* (serialization, formatting)
src/api/middleware/* (auth, logging, utils)
src/backends/traefik/* (template parser, manager, helpers)
config.ts and backendPlugin.ts